fix(dispatch): label-aware concurrency prevents routing-label cancellation#5472
fix(dispatch): label-aware concurrency prevents routing-label cancellation#5472ggallen wants to merge 1 commit into
Conversation
PR Summary by QodoFix dispatch concurrency for labeled events to prevent routing cancellation
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
|
🤖 Review · |
Site previewPreview: https://ed57ef28-site.fullsend-ai.workers.dev Commit: |
4ec46ff to
761aadf
Compare
|
🤖 Finished Review · ✅ Success · Started 4:01 PM UTC · Completed 4:19 PM UTC |
Code Review by Qodo
1. PR label whitelist mismatch
|
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsMedium
Low
Previous run (4)ReviewFindingsMedium
Low
Previous run (5)ReviewFindingsMedium
Low
Previous run (6)ReviewFindingsMedium
Low
Previous run (7)ReviewFindingsMedium
Previous run (8)ReviewFindingsMedium
Previous run (9)ReviewFindingsMedium
Previous run (10)ReviewFindingsMedium
Previous run (11)ReviewFindingsMedium
Previous run (12)ReviewFindingsMedium
Low
Previous run (13)ReviewFindingsMedium
Previous run (14)ReviewFindingsMedium
Low
Previous run (15)ReviewFindingsMedium
Low
Previous run (16)ReviewFindingsMedium
Low
Previous run (17)ReviewFindingsMedium
Low
Labels: PR modifies dispatch workflow routing (concurrency groups and labeled-event filtering) |
761aadf to
e909805
Compare
|
🤖 Finished Review · ✅ Success · Started 4:45 PM UTC · Completed 4:58 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Multi-agent review squad pass (4 reviewers: Claude x2, Grok, Gemini) — no Critical/High findings survived verification. 7 Medium findings posted inline covering: unverified core fix behavior in a live GH Actions environment, a weak test assertion (substring-only, not semantic), a fragile positional test slice, a stale ADR invariant re: per-role concurrency, an ADR update that overgeneralizes which shim gained label-aware concurrency, an unacknowledged "Confidence: Medium" root cause from the source issue, and hand-copied if:/group: blocks with no single source of truth. Two prior review threads (label-whitelist mismatch, weak concurrency test) are already resolved/declared-intentional and not re-flagged.
e909805 to
aed6789
Compare
|
🤖 Finished Review · ✅ Success · Started 7:00 PM UTC · Completed 7:14 PM UTC |
aed6789 to
9ea85ce
Compare
410cd44 to
9aa1082
Compare
9aa1082 to
f93b9de
Compare
|
🤖 Review · |
f93b9de to
e959c59
Compare
|
🤖 Review · |
e959c59 to
27a097a
Compare
|
🤖 Finished Review · ✅ Success · Started 5:59 PM UTC · Completed 6:12 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Multi-agent review (3 agents: Claude ×2, Grok). No Critical/High findings survived verification — the ~15 findings from prior review rounds all check out as genuinely fixed or intentionally deferred, and ifireball's outstanding CHANGES_REQUESTED concern (hard-coded labels → generalization) traces through cleanly to the startsWith('ready-') + harness-dispatch/CEL-trigger mechanism. One new MEDIUM found and posted inline below.
27a097a to
e229395
Compare
|
🤖 Finished Review · ✅ Success · Started 7:38 PM UTC · Completed 7:52 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Multi-agent re-review (4 agents: Claude ×2, Grok, Gemini) of the unlabeled-gap fix. Verified correct by 2 independent agents via full truth-table walks against GitHub's official expression-precedence docs and webhook payload schema, applied byte-identically across all three shim files. One MEDIUM finding (test-coverage gap, not a defect in the fix itself) posted inline below — non-blocking, approving.
Note: one HIGH candidate from this round (claiming unlabeled + ready-* events still waste a spin-up) was ruled a false positive — that's the author's stated deliberate design choice ("routing label removals are still allowed through for future routing support") to avoid over-coupling the shim to dispatch.yml's current routing table, not a defect.
e229395 to
f960712
Compare
|
🤖 Finished Review · ✅ Success · Started 7:55 PM UTC · Completed 8:09 PM UTC |
f960712 to
c7f484c
Compare
|
🤖 Finished Review · ✅ Success · Started 10:06 PM UTC · Completed 10:22 PM UTC |
…ation (fullsend-ai#2452) When multiple labels are applied in a burst, the single concurrency group causes non-routing label events to cancel the routing-label dispatch. Use a label-aware concurrency key that includes the label name for labeled events so each label gets its own slot. Add an if: guard with startsWith to skip non-routing labeled and unlabeled events (those not matching the ready- prefix) at the shim level, avoiding wasted runner spin-ups. Applied to fullsend.yaml (this repo), shim-workflow-call.yaml (per-org template), and shim-per-repo.yaml (per-repo template). Signed-off-by: Greg Allen <greg@fullsend.ai> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>
c7f484c to
f57a254
Compare
|
🤖 Finished Review · ✅ Success · Started 12:00 AM UTC · Completed 12:17 AM UTC |
Summary
Fixes #2452. When triage applies multiple labels rapidly, all
issues/labeledevents shared the same concurrency group (fullsend-dispatch-{number}). GitHub Actions cancels intermediate pending runs, so theready-to-codeevent could be cancelled while a non-routing label (type/bug) survives — the code agent never dispatches.Two complementary fixes:
labeledevents, include the label name in the group soready-to-codeandtype/bugget separate concurrency slots. Non-labeled events share a commondispatchsuffix to preserve existing queuing behavior.if:guard skips the dispatch job entirely for labeled events where the label isn'tready-to-code,ready-for-review, orready-for-triage. Prevents non-routing labels from consuming concurrency slots or wasting runner time.Applied to all three shim files:
.github/workflows/fullsend.yaml(this repo)internal/scaffold/fullsend-repo/templates/shim-workflow-call.yaml(per-org template)internal/scaffold/fullsend-repo/templates/shim-per-repo.yaml(per-repo template,if:guard only — no concurrency group)Test plan
TestShimLabeledEventFiltering— validates all three shims have the routing-label whitelist and the workflow-call shims have label-aware concurrency keysactionlintpasses onfullsend.yamlready-to-codeas a non-final label). Verify theready-to-coderun completes with "Routed to stage: code" rather than being cancelled🤖 Generated with Claude Code